Skip to content

chore(clerk-js): Improve session refresh retry logic - #5397

Merged
panteliselef merged 13 commits into
mainfrom
elef/sdki-949-startstop-session-poller-when-signs-inout
Mar 27, 2025
Merged

chore(clerk-js): Improve session refresh retry logic#5397
panteliselef merged 13 commits into
mainfrom
elef/sdki-949-startstop-session-poller-when-signs-inout

Conversation

@panteliselef

@panteliselef panteliselef commented Mar 19, 2025

Copy link
Copy Markdown
Contributor

Description

This PR performs the following improvements related to how we are refreshing the session token.

  1. Switches from an interval to a timeout. From a parallel behaviour we are switching to a sequential behaviour which allows all retries of particular getToken() call to have concluded before attempting to poll again. Example here.
  2. Clerk.handleUnauthenticated() will set session as null on 500 status code /client response. Avoid infinite request loops, see here.
  3. If /client fails on init, we stop the poller -> create the dummy client -> attempt manual request to /tokens -> start polling again.
  4. retry no longer fires immediately by default.

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

@panteliselef panteliselef self-assigned this Mar 19, 2025
@changeset-bot

changeset-bot Bot commented Mar 19, 2025

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 5b382a3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 19 packages
Name Type
@clerk/clerk-js Minor
@clerk/shared Minor
@clerk/chrome-extension Patch
@clerk/clerk-expo Patch
@clerk/agent-toolkit Patch
@clerk/astro Patch
@clerk/backend Patch
@clerk/elements Patch
@clerk/expo-passkeys Patch
@clerk/express Patch
@clerk/fastify Patch
@clerk/nextjs Patch
@clerk/nuxt Patch
@clerk/react-router Patch
@clerk/clerk-react Patch
@clerk/remix Patch
@clerk/tanstack-react-start Patch
@clerk/testing Patch
@clerk/vue Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel

vercel Bot commented Mar 19, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
clerk-js-sandbox ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 27, 2025 1:57pm

Comment on lines +235 to +248
response = await retry(() => fetch(urlStr, fetchOpts), {
// This retry handles only network errors, not 4xx or 5xx responses,
// so we want to try once immediately to handle simple network blips.
// Since fapiClient is responsible for the network layer only,
// callers need to use their own retry logic where needed.
retryImmediately: true,
// And then exponentially back off with a max delay of 3 seconds.
initialDelay: 700,
maxDelayBetweenRetries: 5000,
shouldRetry: (_: unknown, iterations: number) => {
// We want to retry only GET requests, as other methods are not idempotent.
return overwrittenRequestMethod === 'GET' && iterations < maxTries;
},
});

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Credits to @nikosdouvlis

Comment on lines +95 to +106
// This will retry the getToken call if it fails with a non-4xx error
// We're going to trigger 8 retries in the span of ~3 minutes,
// Example delays: 3s, 5s, 13s, 19s, 26s, 34s, 43s, 50s, total: ~193s
return retry(() => this._getToken(options), {
shouldRetry: (error: unknown, currentIteration: number) => !is4xxError(error) && currentIteration < 4,
factor: 1.55,
retryImmediately: false,
initialDelay: 3 * 1000,
maxDelayBetweenRetries: 50 * 1_000,
jitter: false,
shouldRetry: (error, iterationsCount) => {
return !is4xxError(error) && iterationsCount <= 8;
},

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Credits to @nikosdouvlis

}, INTERVAL_IN_MS);
const run = async () => {
await this.lock.acquireLockAndRun(cb);
this.timerId = this.workerTimers.setTimeout(run, INTERVAL_IN_MS);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Credits to @nikosdouvlis

Comment on lines +2070 to +2083
/**
* In most scenarios we want the poller to stop while we are fetching a fresh token during an outage.
* We want to avoid having the below `getToken()` retrying at the same time as the poller.
*/
this.#authService?.stopPollingForToken();

// Attempt to grab a fresh token
await this.session
?.getToken({ skipCache: true })
// If the token fetch fails, let Clerk be marked as loaded and leave it up to the poller.
.catch(() => null)
.finally(() => {
this.#authService?.startPollingForToken();
});

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid parallel retries from this explicit invocation and the poller.

Comment on lines +18 to +23
const run = async () => {
await this.lock.acquireLockAndRun(cb);
this.timerId = this.workerTimers.setTimeout(run, INTERVAL_IN_MS);
};

void run();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reviewers:
This change allows for this behaviour

getToken -> 500 -> retry... retry.. retry... -> success/failure -> getToken

Until now, we had this one, which is no longer desired.

getToken -> 500 -> retry... retry.. retry... -> success/failure -> getToken
                                        getToken -> 500 -> retry... retry.. retry... -> success/failure -> getToken
                                                                       getToken -> 500 -> retry... retry.. retry... -> success/failure -> getToken

Comment on lines +1695 to +1699
// `/client` can fail with either a 401, a 403, 500 or network errors.
// 401 is already handled internally in our fetcher.
// 403 means that the client is blocked, signing out the user is the only option.
// 500 means that the client is not working, signing out the user is the only option, since the intention was to sign out the user.
if (isClerkAPIResponseError(err) && [403, 500].includes(err.status)) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the needed ?
When /client fails with 500, we attempt to read from __session and call /tokens. If /tokens returns with a 4xx handleUnauthencated gets called which tries to reload client which will fail with 500.

At this point the user cannot really do anything to resolve this, and clerk.js cannot auto recover. We set session: null to stop the poller from falling into an infinite loop.

Comment thread packages/clerk-js/src/core/fapiClient.ts Outdated
…poller-when-signs-inout' into elef/sdki-949-startstop-session-poller-when-signs-inout
@panteliselef
panteliselef marked this pull request as ready for review March 21, 2025 16:30
@panteliselef
panteliselef requested a review from a team March 21, 2025 16:30
…signs-inout

# Conflicts:
#	packages/clerk-js/bundlewatch.config.json
Comment thread packages/clerk-js/src/core/resources/Session.ts Outdated
* Controls whether the helper should retry the operation immediately once before applying exponential backoff.
* The delay for the immediate retry is 100ms.
* @default true
* @default false

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how come we changed the default here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We settled on it, as the best default to have.

@panteliselef

Copy link
Copy Markdown
Contributor Author

!snapshot

@clerk-cookie

Copy link
Copy Markdown
Collaborator

Hey @panteliselef - the snapshot version command generated the following package versions:

Package Version
@clerk/agent-toolkit 0.0.16-snapshot.v20250327150215
@clerk/astro 2.4.5-snapshot.v20250327150215
@clerk/backend 1.25.8-snapshot.v20250327150215
@clerk/chrome-extension 2.2.23-snapshot.v20250327150215
@clerk/clerk-js 5.59.0-snapshot.v20250327150215
@clerk/elements 0.23.8-snapshot.v20250327150215
@clerk/clerk-expo 2.9.6-snapshot.v20250327150215
@clerk/expo-passkeys 0.2.0-snapshot.v20250327150215
@clerk/express 1.3.59-snapshot.v20250327150215
@clerk/fastify 2.1.32-snapshot.v20250327150215
@clerk/localizations 3.13.4-snapshot.v20250327150215
@clerk/nextjs 6.12.12-snapshot.v20250327150215
@clerk/nuxt 1.4.6-snapshot.v20250327150215
@clerk/clerk-react 5.25.5-snapshot.v20250327150215
@clerk/react-router 1.1.11-snapshot.v20250327150215
@clerk/remix 4.5.11-snapshot.v20250327150215
@clerk/shared 3.3.0-snapshot.v20250327150215
@clerk/tanstack-react-start 0.12.2-snapshot.v20250327150215
@clerk/testing 1.4.33-snapshot.v20250327150215
@clerk/themes 2.2.26-snapshot.v20250327150215
@clerk/types 4.50.1-snapshot.v20250327150215
@clerk/vue 1.4.5-snapshot.v20250327150215

Tip: Use the snippet copy button below to quickly install the required packages.
@clerk/agent-toolkit

npm i @clerk/agent-toolkit@0.0.16-snapshot.v20250327150215 --save-exact

@clerk/astro

npm i @clerk/astro@2.4.5-snapshot.v20250327150215 --save-exact

@clerk/backend

npm i @clerk/backend@1.25.8-snapshot.v20250327150215 --save-exact

@clerk/chrome-extension

npm i @clerk/chrome-extension@2.2.23-snapshot.v20250327150215 --save-exact

@clerk/clerk-js

npm i @clerk/clerk-js@5.59.0-snapshot.v20250327150215 --save-exact

@clerk/elements

npm i @clerk/elements@0.23.8-snapshot.v20250327150215 --save-exact

@clerk/clerk-expo

npm i @clerk/clerk-expo@2.9.6-snapshot.v20250327150215 --save-exact

@clerk/expo-passkeys

npm i @clerk/expo-passkeys@0.2.0-snapshot.v20250327150215 --save-exact

@clerk/express

npm i @clerk/express@1.3.59-snapshot.v20250327150215 --save-exact

@clerk/fastify

npm i @clerk/fastify@2.1.32-snapshot.v20250327150215 --save-exact

@clerk/localizations

npm i @clerk/localizations@3.13.4-snapshot.v20250327150215 --save-exact

@clerk/nextjs

npm i @clerk/nextjs@6.12.12-snapshot.v20250327150215 --save-exact

@clerk/nuxt

npm i @clerk/nuxt@1.4.6-snapshot.v20250327150215 --save-exact

@clerk/clerk-react

npm i @clerk/clerk-react@5.25.5-snapshot.v20250327150215 --save-exact

@clerk/react-router

npm i @clerk/react-router@1.1.11-snapshot.v20250327150215 --save-exact

@clerk/remix

npm i @clerk/remix@4.5.11-snapshot.v20250327150215 --save-exact

@clerk/shared

npm i @clerk/shared@3.3.0-snapshot.v20250327150215 --save-exact

@clerk/tanstack-react-start

npm i @clerk/tanstack-react-start@0.12.2-snapshot.v20250327150215 --save-exact

@clerk/testing

npm i @clerk/testing@1.4.33-snapshot.v20250327150215 --save-exact

@clerk/themes

npm i @clerk/themes@2.2.26-snapshot.v20250327150215 --save-exact

@clerk/types

npm i @clerk/types@4.50.1-snapshot.v20250327150215 --save-exact

@clerk/vue

npm i @clerk/vue@1.4.5-snapshot.v20250327150215 --save-exact

@panteliselef
panteliselef merged commit e984494 into main Mar 27, 2025
@panteliselef
panteliselef deleted the elef/sdki-949-startstop-session-poller-when-signs-inout branch March 27, 2025 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants